Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add = character to tag validation regex #2008

Merged
merged 1 commit into from
Oct 12, 2024
Merged

add = character to tag validation regex #2008

merged 1 commit into from
Oct 12, 2024

Conversation

cvignal
Copy link
Contributor

@cvignal cvignal commented Oct 11, 2024

The tags restrictions mentioned for the validTagKeyValue regex includes the = character which is not present in the regex.

This PR aims at reducing this drift between AWS and Minio restrictions on this API.
I've added a simple test case in the unit test for this package, let me know if this enough or if further testing is needed.

@harshavardhana
Copy link
Member

= is only supported in EC2 not in S3 tags, they don't document it properly.

The regex we have is well tested against AWS S3.

Please provide more evidence, test it with S3 and then let us reopen this PR if needed.

@cvignal
Copy link
Contributor Author

cvignal commented Oct 11, 2024

As per the doc linked in the package:

The allowed characters across all AWS services are: letters (a-z, A-Z), numbers (0-9), and spaces representable in UTF-8, and the following characters: + - = . _ : / @.

In our organization we compute md5 hashes (which can contain =) and store them in tags on S3 so I'm 100% positive that this character is allowed in S3 tags.
I agree that the documentation for S3 is lacking in this regard, I'll open a support case to know the exact rules for validation.

@fleuryc-owkin
Copy link

@harshavardhana I confirm : I just successfully manually added a tag containing = to an S3 object via the console.

@harshavardhana
Copy link
Member

Please provide a code sample

@klauspost
Copy link
Contributor

When relaxing minio-go restrictions, using mc:

λ go build&&mc tag set --debug mys3/klauspost/test.csv "key=123="
XML:
<Tagging><TagSet><Tag><Key>key</Key><Value>123=</Value></Tag></TagSet></Tagging>
...

mc: <DEBUG> PUT /test.csv?tagging= HTTP/1.1
Host: klauspost.s3.dualstack.us-east-2.amazonaws.com
User-Agent: MinIO (windows; amd64) minio-go/v7.0.77 mc/DEVELOPMENT.GOGET
Content-Length: 80
Accept-Encoding: zstd,gzip
Authorization: AWS4-HMAC-SHA256 Credential=**REDACTED**/20241012/us-east-2/s3/aws4_request, SignedHeaders=content-md5;host;x-amz-content-sha256;x-amz-date, Signature=**REDACTED**
Content-Md5: eqvfVX+z+k3vxTJ6y9/6+w==
X-Amz-Content-Sha256: UNSIGNED-PAYLOAD
X-Amz-Date: 20241012T090131Z

mc: <DEBUG> HTTP/1.1 200 OK
Content-Length: 0
Date: Sat, 12 Oct 2024 09:01:31 GMT
Server: AmazonS3
X-Amz-Id-2: P/u8pn/Q7aFnlKrF7Vy36BrUe4JHZNDiAF2FBLSkg32AiNbj3qEwijahUujyQKdJOc8HN+4+wIWcbp9sSQE+qg==
X-Amz-Request-Id: 1384EV8JC4VX1HVY
X-Amz-Version-Id: null

...

Tags set for https://s3.amazonaws.com/klauspost/test.csv.

λ go build&&mc tag list --debug mys3/klauspost/test.csv
...
mc: <DEBUG> GET /test.csv?tagging= HTTP/1.1
Host: klauspost.s3.dualstack.us-east-2.amazonaws.com
User-Agent: MinIO (windows; amd64) minio-go/v7.0.77 mc/DEVELOPMENT.GOGET
Accept-Encoding: zstd,gzip
Authorization: AWS4-HMAC-SHA256 Credential=**REDACTED**/20241012/us-east-2/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature=**REDACTED**
X-Amz-Content-Sha256: UNSIGNED-PAYLOAD
X-Amz-Date: 20241012T090434Z

mc: <DEBUG> HTTP/1.1 200 OK
Transfer-Encoding: chunked
Date: Sat, 12 Oct 2024 09:04:34 GMT
Server: AmazonS3
X-Amz-Id-2: BxvuQLihSZM22S8fbdipp6B+bIxfKex76CFvigSmQyMjYA2FTOibqYtelTEQEmqf8xtalmB3gGSPTJx5mlqbzJlCU2BxvVfRhQ3sJGmQXa0=
X-Amz-Request-Id: R38TVC03WY2HBMS3
X-Amz-Version-Id: null

...

<?xml version="1.0" encoding="UTF-8"?>
<Tagging xmlns="http://s3.amazonaws.com/doc/2006-03-01/"><TagSet><Tag><Key>key</Key><Value>123=</Value></Tag></TagSet></Tagging>

Name : https://s3.amazonaws.com/klauspost/test.csv
key  : 123=

@klauspost klauspost reopened this Oct 12, 2024
@harshavardhana
Copy link
Member

λ go build&&mc tag set --debug mys3/klauspost/test.csv "key=123="

can you try = in the beginning of the string key==123 ?

@klauspost
Copy link
Contributor 

λ go build&&mc tag set --debug mys3/klauspost/test.csv "key2==123"
XML:
<Tagging><TagSet><Tag><Key>key2</Key><Value>=123</Value></Tag></TagSet></Tagging>
...
mc: <DEBUG> PUT /test.csv?tagging= HTTP/1.1
Host: klauspost.s3.dualstack.us-east-2.amazonaws.com
User-Agent: MinIO (windows; amd64) minio-go/v7.0.77 mc/DEVELOPMENT.GOGET
Content-Length: 81
Accept-Encoding: zstd,gzip
Authorization: AWS4-HMAC-SHA256 Credential=**REDACTED**/20241012/us-east-2/s3/aws4_request, SignedHeaders=content-md5;host;x-amz-content-sha256;x-amz-date, Signature=**REDACTED**
Content-Md5: fHsUDJyytW3UmybNiuZSsg==
X-Amz-Content-Sha256: UNSIGNED-PAYLOAD
X-Amz-Date: 20241012T093059Z

mc: <DEBUG> HTTP/1.1 200 OK
Content-Length: 0
Date: Sat, 12 Oct 2024 09:30:59 GMT
Server: AmazonS3
X-Amz-Id-2: 1TBaO3Lon0fCK9rewdSdNo5AbuDs5ejvDUUroNJN73GXAyvKrzlw24Av59nbyPISPSFWtKpQZGw+HBTZyQKFOOHJsDGH55ZD8NjG0ZyOJi8=
X-Amz-Request-Id: XYAT0X8YV5TCZBHD
X-Amz-Version-Id: null

λ go build&&mc tag list mys3/klauspost/test.csv
<?xml version="1.0" encoding="UTF-8"?>
<Tagging xmlns="http://s3.amazonaws.com/doc/2006-03-01/"><TagSet><Tag><Key>key2</Key><Value>=123</Value></Tag></TagSet></Tagging>
Name : https://s3.amazonaws.com/klauspost/test.csv
key2 : =123

@harshavardhana harshavardhana merged commit 95a7dde into minio:master Oct 12, 2024
5 checks passed
@cvignal cvignal deleted the chore/update-tag-validation-regex branch October 23, 2024 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@harshavardhana harshavardhana harshavardhana approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cvignal @harshavardhana @fleuryc-owkin @klauspost